Skip to content

refactor(cosmo_val): god-class → tested, primitive-backed package#207

Merged
cailmdaley merged 42 commits into
developfrom
refactor/cosmo-val-package
Jun 30, 2026
Merged

refactor(cosmo_val): god-class → tested, primitive-backed package#207
cailmdaley merged 42 commits into
developfrom
refactor/cosmo-val-package

Conversation

@cailmdaley

@cailmdaley cailmdaley commented Jun 20, 2026

Copy link
Copy Markdown
Collaborator

What

Turns the 3,700-line CosmologyValidation god-class into a clean, tested cosmo_val/ package backed by shared primitive modules. Behavior-preserving throughout, except three clearly-committed bug fixes (below). Built in reviewable layers — review per commit or per section:

  1. Package split. cosmo_val.py → a cosmo_val/ mixin package, one module per diagnostic (pseudo_cl, psf_systematics, real_space, catalog_characterization, pure_eb, cosebis, core). CosmologyValidation composes the mixins; every call site is unchanged. The 85 methods are conserved exactly.
  2. Cleanup. Per-module tightening plus cross-module dedup (~−150 lines): _output_path/_binning/_cprint helpers, get_cov_from_onecov folded into statistics.cov_from_one_covariance, chi2_and_pte standardized to solve+sf.
  3. Thinning. Stateless computation pulled out of the orchestrator into shared primitives: the survey-stat family → survey.py, and the NaMaster estimator → a new top-level pseudo_cl.py. glass_mock's drifted estimator copies now route through the shared primitives.
  4. New test coverage. test_pseudo_cl.py adds the first numeric pins for the pseudo-Cℓ estimator (a Paper II deliverable that had none) — golden values for the three binning schemes, the occupancy map, and the map/catalog EE·EB·BB spectra.

The architecture this lands

cosmo_val/ is the stateful orchestrator (holds config/paths/results, loads catalogs, sequences diagnostics, caches, plots, saves). The top-level modules (b_modes, rho_tau, cosmology, statistics, survey, pseudo_cl) are the stateless primitives that analysis scripts also import directly — which is why they live outside the class.

Bugs fixed (each its own commit)

  • demo_create_footprint_mask called cosmo_val.hsp_map_logical_or, which lives in plots.py — repointed.
  • Aperture-mass DES calibration used scalar R for every version while 2pcf used the catalog-averaged R11/R22; unified so DES is calibrated the same way in both.
  • Pseudo-Cℓ noise-debiasing was non-reproducibleapply_random_rotation called np.random.seed() with no argument, re-seeding from entropy every call. Now threads a generator seeded from a new CosmologyValidation.cell_seed (default 8192): same Monte-Carlo scheme, reproducible run-to-run.

Verification

ruff check src/sp_validation clean; all modules import (the relative-import depth is exercised, not just linted). Value-drift (test_cosmo_val, test_b_modes) green; the new test_pseudo_cl pins pass; full non-slow suite: 136 passed.

Stacked on #206. Draft for review.

— Claude on behalf of Cail

cailmdaley and others added 28 commits June 20, 2026 04:50
The module named "basic" was a grab-bag: the 546-line `metacal` response
class (the heart of shear calibration) plus galaxy-selection masks and a
handful of cosmology-independent statistics helpers — none of which "basic"
described. Its symbols now live where they belong, and basic.py is deleted.

- `metacal` class + `mask_gal_size`/`mask_gal_SNR` (galaxy selection) →
  calibration.py, joining the m/c routines that already consumed a
  `gal_metacal` instance. One subsystem, one module.
- `jackknif_weighted_average2`, `corr_from_cov`, `chi2_and_pte`,
  `cov_from_one_covariance` → new statistics.py (a clean leaf: numpy/scipy
  only; calibration imports the jackknife from it).
- Every importer repointed (papers, scripts, the two scratch/guerrini
  import lines — path-only, his logic untouched); dead `from sp_validation
  import basic` lines removed from calibration.py and cat.py; `__all__` and
  the architecture docs updated.
- Tests split: metacal + mask pins → test_calibration.py, jackknife pin →
  test_statistics.py; test_basic.py removed.

All moved code is byte-identical to the original (md5-verified); value-drift
pins (metacal R-matrix rtol 1e-12) and the full suite pass in-container,
except the pre-existing galaxy/cs_util.size old-sandbox gap. No circular
imports. Verified by an adversarial multi-agent pass (byte-identity,
no-stale-refs, value-pins, no-cycles).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
scratch/guerrini/ and the namaster_utils→source / Gaussian-sims work he
reserved for his next PR in the #197 review. So future workers don't touch it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ation

Follow-up polish on the basic.py dissolution (#199).

Tests — characterization (value-drift) coverage for the three statistics.py
helpers that had none, with literals generated by running the real functions
in-container and teeth on each:
- corr_from_cov: unit diagonal + reconstruction from cov/outer(std,std)
- chi2_and_pte: diagonal reduces to sum((d/sigma)^2) with matching scipy PTE,
  plus a non-diagonal case exercising the full d^T C^-1 d path
- cov_from_one_covariance: gaussian(col 10) vs non-gaussian(col 9) selection
  and a row-major-layout check (a transpose would be caught)

Calibration — strictly behavior-preserving dead-code removal:
- 3 unused module imports (util, io, get_footprint — verified unreferenced)
- an unused local (col_noshear) in metacal._read_data
- the uncallable metacal._return method (defined without self, references
  self.* in its body — would NameError if ever invoked; referenced nowhere)

Value pins (metacal R-matrix, m/c bias) stay green; conservatively skipped any
change that would reorder float ops or restructure an estimator. Verified by an
adversarial behavior-preservation review.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
info.py had zero importers; its only content was a redundant
__name__ = 'sp_validation'. cat.py imported __version__ and __name__
from the package root but used only __version__ (line 607); the
software name at line 606 is already hardcoded. Repoint to the
canonical home: from sp_validation.version import __version__.

Register the retired import path in the dangling-move guard.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The old __all__ listed modules nobody imports through the package
(io, plot_style, cosmo_val) and omitted the two genuinely public
diagnostic modules rho_tau and b_modes. Replace it with the real
public surface, alphabetised, and drop the stale commented-out
explicit-import block. Nothing does `from sp_validation import *`,
so this is purely a documentation fix.

(util and run_joint_cat are renamed in the Tier-2 commits.)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Five methods each imported from .b_modes inside their bodies. b_modes
is import-time side-effect-light (it pulls only .cosmology) and has no
back-edge to cosmo_val, so the locals were defensive, not necessary.
Consolidate the union of imported names into one top-level block next
to the existing cosmology/rho_tau imports and drop the five inline
imports. test_cosmo_val: 11 passed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two module-level defs named confusion_matrix existed; the first
(mask, confidence_level=0.9) was a near-duplicate of correlation_matrix
and was unconditionally shadowed by the second
(prediction, observation) ~40 lines later. Every caller — in
scripts/calibration and scripts/examples — uses the
(prediction, observation) signature. Remove the dead first def.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Five functions had zero callers anywhere in the repo (src, scripts,
papers, scratch, notebooks), including across the star-imports of
plots:
  cosmology.py: get_clusters, stack_mm3, gamma_T_tc, xi_gal_gal_tc
  plots.py:     plot_map_stacked
Removing the cosmology block also orphaned the imports that existed
solely for it (treecorr, fits, canfar, radec2xy, cKDTree, tqdm,
get_footprint); drop those too. test_cosmology + test_plots: 29 passed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
util.py held only millify / print_millified — number formatting, not a
grab-bag. Rename it to format.py and sweep all importers:
  internal: cat.py, run_joint_cat.py (util.millify -> format.millify)
  scripts:  apply_alpha.py, examples/demo_calibrate_minimal_cat.py,
            calibration/extract_info.py (star-import x2)
  papers:   catalog/hist_mag.py
Register the rename in the dangling-move guard; update package __all__.

B1: scripts/plot_leakage.py imported transform_nan from the old util
module — a symbol removed from the library long ago and never used in
the script. Drop the dead import.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
SquareRootScale is a matplotlib ScaleBase subclass — plotting
infrastructure, not rho/tau logic. Move the class and its
register_scale call into plots.py (which now carries the
matplotlib.scale/ticker/transforms imports it needs). rho_tau.py keeps
a compat re-export so existing
`from sp_validation.rho_tau import SquareRootScale` callers — in
cosmo_inference, scratch/guerrini, papers/harmonic — still resolve.

workflow/scripts/plotting_utils.py holds a near-duplicate that
diverges (ScalarFormatter(useMathText=True); inverted transform method
named transform_non_affine not transform), so it is left in place.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Six harmonic-space scripts imported SquareRootScale from
sp_validation.utils_cosmo_val, a module that no longer exists. Repoint
to sp_validation.rho_tau (the compat re-export), matching the sibling
2026_03_17 script. get_params_rho_tau was already correctly imported
from rho_tau. No other utils_cosmo_val imports remain (the two
scratch/guerrini mentions are prose noting the module's removal).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The module holds the catalogue-builder runner classes (JointCat,
ApplyHspMasks, CalibrateCat) and their run_* entry-point functions;
catalog_builders names that role. Sweep all importers (the
`as sp_joint` alias is preserved, only the module name changes):
  scripts/apply_hsp_masks.py
  scripts/examples/{demo_check_footprint, create_binned_mask_comprehensive,
    demo_comprehensive_to_minimal_cat, demo_create_footprint_mask,
    demo_calibrate_minimal_cat}.py
  scripts/calibration/{create_joint_comprehensive_cat (direct symbol),
    demo_apply_hsp_masks, calibrate_comprehensive_cat}.py
  scratch/kilbinger/demo_binned_mask.py
  papers/catalog/hist_mag.py
Also update the two prose references in docs and update __all__ and
the dangling-move guard.

The OPTIONAL masks.py extraction (Mask + mask-algebra fns) is deferred:
those symbols are reached externally through the sp_joint.* module
alias (Mask, get_masks_from_config, print_mask_stats across 4 scripts +
papers/catalog), so splitting them out would require either re-exports
or sweeping the public call surface — beyond a behavior-preserving move.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Move the healsparse-backed spatial-masking cluster out of
catalog_builders.py into a dedicated masks.py: the Mask class plus
get_masks_from_config, print_mask_stats, correlation_matrix, and
confusion_matrix. Bodies are byte-identical; the move carries the
numexpr/scipy.stats imports those helpers need (now removed from
catalog_builders, which no longer references them).

catalog_builders.py re-exports the five symbols from sp_validation.masks
so external code using `from sp_validation import catalog_builders as
sp_joint` keeps resolving sp_joint.Mask, sp_joint.get_masks_from_config,
etc. The *Cat runner classes (ApplyHspMasks, ReadCat, run_* entry points)
stay; ApplyHspMasks uses healsparse directly, not the Mask class.

masks added to __init__.__all__. No MOVE_MAP entry: this is an
extraction-in-place (catalog_builders survives), not a retired path.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The catalogue module pair now reads by role: catalog.py is the data
layer (read/write/column-access/matching free functions), catalog_builders.py
is the construction pipeline (runner classes built on it). Module docstrings
state this hierarchy explicitly.

Behaviour-preserving. Every importer of the local sp_validation.cat module is
swept to sp_validation.catalog, each preserving its local binding (bare
`import cat` forms gain `as cat` so function bodies are unchanged). The
cs_util `cat` import is a different module and is left untouched throughout.

The dangling-reference guard registers the retired flat-import form
`sp_validation.cat import` rather than the bare `sp_validation.cat` token,
which would false-positive on the live `sp_validation.catalog` /
`sp_validation.catalog_builders` modules (same prefix trap as glass_mock).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…aper analysis scripts

The gate revealed peripheral residual the per-file-ignore globs didn't reach
(analysis scripts under papers/<paper>/*.py, cosmo_inference notebooks) plus the
Snakemake-injected `snakemake` global (F821). Region-aware decision: enforce lint
on src/ only (matching the pre-commit hook), keep format repo-wide, run the
repo-wide lint advisory. src/ stays pristine (ruff check src/ => clean).
…-export)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Converting cosmo_val from a module to a package moved every file one level
deeper, so single-dot relative imports to top-level siblings now resolve
inside the package. Point them up a level:
- ..b_modes (core, pure_eb, cosebis)
- ..cosmology (core, pseudo_cl)
- ..rho_tau (psf_systematics, pseudo_cl)

Re-export cs_plots in __init__ from its origin (cs_util.plots) rather than
from core, which no longer needs the alias after the plotting methods moved
to their mixins. Preserves the sp_validation.cosmo_val.cs_plots attribute path.

Package now imports cleanly; test_cosmo_val + test_b_modes: 18 passed, 1 skipped.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…module

The demo called cosmo_val.hsp_map_logical_or, but that function has only ever
lived in plots.py — the call raised AttributeError. Import it from its home.
(Pre-existing bug surfaced by the cosmo_val package split.)
…eserving)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…m_one_covariance directly

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
cailmdaley and others added 11 commits June 21, 2026 00:40
Add a private CosmologyValidation._output_path(*parts) that returns
os.path.abspath(os.path.join(self.cc["paths"]["output"], *parts)), and
route the 29 abspath-wrapped output-path constructions across the mixin
modules (catalog_characterization, pseudo_cl, psf_systematics,
real_space) through it.

Behavior-preserving: only the os.path.abspath(f"{output}/...") sites are
converted, where abspath(join(base, suffix)) is byte-identical to
abspath(f"{base}/{suffix}") for every suffix shape in the package
(verified, incl. embedded subdirs and trailing slashes). Raw f-strings
that are not abspath'd (psf_systematics output_dir handoffs to
shear_psf_leakage) are intentionally left as-is to avoid changing the
string they pass to external tools.

Value-drift gate green: 18 passed, 1 skipped.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… it (re-bless pins ~ULP)

chi2_and_pte now computes chi2 via np.linalg.solve (not forming C^-1) and
PTE via stats.chi2.sf (not 1 - cdf), the numerically proper forms: solve is
more stable/cheaper than an explicit inverse, and the survival function
avoids catastrophic cancellation in the low-PTE tail.

The two hand-inlined C_l^BB chi2/PTE sites in cosmo_val (core.summarize_bmodes,
pseudo_cl.plot_pseudo_cl) already used solve+sf, so they now route through the
single chi2_and_pte primitive with no numerical change; their local scipy
imports are dropped. No pinned value shifted (the value-drift pins flow through
the independent b_modes Hartlap path), so no re-blessing was required.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…r calibration

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…per caller

The shared helper applied the DES R11/R22 branch to both callers, but
calculate_aperture_mass_dispersion historically used scalar R for every
version. Add a des_branch flag so each caller keeps its exact prior behavior
(2pcf: DES branch; aperture-mass: scalar R). The asymmetry is flagged in the
docstring as a likely latent bug for a deliberate, separate fix.
calculate_aperture_mass_dispersion used scalar R for every version, including
DES, while calculate_2pcf used the catalog-averaged R11/R22 for DES. That
asymmetry was a latent bug; _calibrated_g now applies the DES branch
identically for both callers. Untested path (no DES fixture) — value-drift
suite unaffected and green.
…n the mixin

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…_cl.py, thin the mixin

Move the stateless pseudo-Cl computation out of PseudoClMixin into named free
functions in a new top-level sp_validation/pseudo_cl.py (mirrors b_modes.py /
rho_tau.py): make_namaster_bin, get_n_gal_map (weights kwarg, subsumes the
glass_mock unweighted twin), apply_random_rotation (rng injectable; default
preserves the no-arg np.random.seed() noise-debiasing behavior), and the
map-/catalog-based estimators get_pseudo_cls_map / get_pseudo_cls_catalog.
pseudo_cl_geometry centralizes the (lmin, lmax, b_lmax) triple.

The mixin methods are now thin wrappers (load via self -> call primitive),
public names/signatures unchanged so call sites do not move. glass_mock's
get_n_gal_map delegates to the primitive via a lazy import, preserving its
CAMB-only import-time independence from the harmonic stack.

Pins (test_pseudo_cl.py) green; value-drift gate (test_cosmo_val + test_b_modes)
18 passed / 1 skipped; glass_mock tests green; actual cross-module import
verified in the container.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…opies

Repoint the GLASS-mock harmonic stats at the new src/sp_validation/pseudo_cl.py
primitives so the bandpower binning can no longer drift from the estimator the
rest of the package uses.

- powspace_bins: deleted. Its square-root bandpower spacing is exactly
  pseudo_cl.make_namaster_bin(..., "powspace", power=0.5) on the
  pseudo_cl_geometry(nside) range. Replaced by a thin _mock_powspace_bin wrapper
  that returns the same (bin, ell_eff, lmax, b_lmax) 4-tuple the mock helpers
  consume, now sourced from the shared primitive. Verified bitwise-identical
  binning (ell_eff and per-bin ell membership) across nside/lmin/n_bins configs.
- get_n_gal_map: already delegates to the weighted primitive with weights=None
  (unweighted counts); left as the count-default twin used by the map estimator.
- compute_two_point_cl / compute_two_point_cl_map: kept their own NaMaster field
  construction (documented) — the mock field is UNWEIGHTED and these return the
  COUPLED pseudo-Cl alongside the decoupled one with the ell axis prepended,
  which the primitives' (ell_eff, cl_all, wsp) contract does not expose. They now
  take their binning and galaxy-count map from the shared primitives.

Value-drift gate green (18 passed, 1 skipped); full non-slow suite 136 passed,
1 skipped, 1 xpassed, only the two known-environmental failures
(test_bmodes_workflow_dry_runs, test_configured_paths_exist_on_candide).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
apply_random_rotation called np.random.seed() with no argument, re-seeding
from OS entropy on every call, so the pseudo-Cl noise-debiasing realizations
were non-reproducible run-to-run. Drop the bare seed; the primitive now uses
its rng argument (default_rng() when None), and the debiasing + covariance
sampling paths thread a generator seeded from a new CosmologyValidation
cell_seed (default 8192). Same numerical scheme, now reproducible. The
test_pseudo_cl reproducibility test replaces the old non-determinism guard.
@cailmdaley cailmdaley changed the title refactor(cosmo_val): split CosmologyValidation god-class into a mixin package refactor(cosmo_val): god-class → tested, primitive-backed package Jun 21, 2026
@cailmdaley cailmdaley requested review from martinkilbinger and sachaguer and removed request for martinkilbinger June 23, 2026 15:03
@cailmdaley

Copy link
Copy Markdown
Collaborator Author

@sachaguer you may want to take a look at the pseudo_cl random seed behavior and where to integrate in the namaster utils, although the latter doesn't have to be part of this PR!

@sachaguer sachaguer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked the seeding for the pseudo_cl and it looks fine to me.
Some of the namaster_utils are in pseudo_cl.py. Functions could still be broken into more elementary pieces but it is good as is and can be kept for a future PR.

Base automatically changed from chore/ruff to develop June 30, 2026 14:51
@cailmdaley cailmdaley marked this pull request as ready for review June 30, 2026 14:53
…licts)

#206 (ruff gate) and #199 (basic.py dissolve) squash-merged to develop while
#207 still carried their pre-squash commits, so this merge collapses the stack.
Resolved by category:

- develop's side where it carried fixes #207 lacked: Sacha's cov_sim_gaussian
  removal in papers/harmonic/2025_09_11_validation_cov_cell.py (21c7d96), the
  consistency-notebook deletion (moved to scratch), the canonical #206
  lint.yml / pre-commit enforcement model, the broader cosmo_inference/** E402
  per-file-ignore.
- #207's side for its genuine work: the cosmo_val/ package (flat cosmo_val.py
  deleted), the pseudo_cl primitives, survey.area_from_coords, chi2_and_pte
  solve+sf (pins re-blessed for it), the glass_mock dedup, the demo
  hsp_map_logical_or fix; dropped develop's duplicate SquareRootScale.

Verified on the fresh container: ruff format+check clean repo-wide;
CosmologyValidation imports (75 methods); value-drift + golden pins
32 passed / 1 skipped.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jmk58xivawGCVFgejJJeDp
@cailmdaley cailmdaley merged commit 2c3e7ba into develop Jun 30, 2026
3 checks passed
@cailmdaley cailmdaley deleted the refactor/cosmo-val-package branch June 30, 2026 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Decompose run_cosmo_val into a modular workflow (split the cosmo_val god class)

2 participants